[SPARK-1745] Move interrupted flag from TaskContext constructor (minor) #675
Conversation
andrewor14
changed the title
|
Merged build triggered. |
|
Merged build started. |
|
Merged build finished. |
|
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/14754/ |
|
Failed compilation is unrelated. Waiting for the fix in #673 to go in. |
|
Jenkins, retest this please. |
|
Merged build triggered. |
|
Merged build started. |
|
Merged build finished. |
|
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/14763/ |
|
Jenkins, retest this please. |
|
Merged build triggered. |
|
Merged build started. |
|
Merged build finished. |
|
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/14767/ |
|
Merged build triggered. |
|
Merged build started. |
|
Would you mind removing the Try in ShuffleMapTask which I should've removed in #640 but failed to actually push? <3 |
Conflicts: core/src/main/scala/org/apache/spark/TaskContext.scala
|
Merged build triggered. |
|
Merged build started. |
|
Merged build finished. All automated tests passed. |
|
All automated tests passed. |
|
Merged build finished. All automated tests passed. |
|
All automated tests passed. |
| @@ -58,6 +60,6 @@ class TaskContext( | |||
| def executeOnCompleteCallbacks() { | |||
| completed = true | |||
| // Process complete callbacks in the reverse order of registration | |||
| onCompleteCallbacks.reverse.foreach{_()} | |||
| onCompleteCallbacks.reverse.foreach{ _() } | |||
There was a problem hiding this comment.
super nit, could we put a space after foreach?
There was a problem hiding this comment.
oh yeah, idk how I missed that
|
Merged build triggered. |
|
Merged build started. |
|
Merged build finished. All automated tests passed. |
|
All automated tests passed. |
|
This should be ready for merge |
|
LGTM, merging into master and 1.0. Thanks! |
It makes little sense to start a TaskContext that is interrupted. Indeed, I searched for all use cases of it and didn't find a single instance in which `interrupted` is true on construction. This was inspired by reviewing #640, which adds an additional `@volatile var completed` that is similar. These are not the most urgent changes, but I wanted to push them out before I forget. Author: Andrew Or <[email protected]> Closes #675 from andrewor14/task-context and squashes the following commits: 9575e02 [Andrew Or] Add space 69455d1 [Andrew Or] Merge branch 'master' of github.com:apache/spark into task-context c471490 [Andrew Or] Oops, removed one flag too many. Adding it back. 85311f8 [Andrew Or] Move interrupted flag from TaskContext constructor (cherry picked from commit c3f8b78) Signed-off-by: Aaron Davidson <[email protected]>
It makes little sense to start a TaskContext that is interrupted. Indeed, I searched for all use cases of it and didn't find a single instance in which `interrupted` is true on construction. This was inspired by reviewing apache#640, which adds an additional `@volatile var completed` that is similar. These are not the most urgent changes, but I wanted to push them out before I forget. Author: Andrew Or <[email protected]> Closes apache#675 from andrewor14/task-context and squashes the following commits: 9575e02 [Andrew Or] Add space 69455d1 [Andrew Or] Merge branch 'master' of github.com:apache/spark into task-context c471490 [Andrew Or] Oops, removed one flag too many. Adding it back. 85311f8 [Andrew Or] Move interrupted flag from TaskContext constructor
* Bump Jackson version * Update manifest
It makes little sense to start a TaskContext that is interrupted. Indeed, I searched for all use cases of it and didn't find a single instance in which
interruptedis true on construction.This was inspired by reviewing #640, which adds an additional
@volatile var completedthat is similar. These are not the most urgent changes, but I wanted to push them out before I forget.